Skip to content

Fix doctrine/dbal 2.13 incompatibility #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 20, 2021
Merged

Fix doctrine/dbal 2.13 incompatibility #64

merged 1 commit into from
Apr 20, 2021

Conversation

acrobat
Copy link
Contributor

@acrobat acrobat commented Apr 5, 2021

Fixes #63

As this blogpost suggests, libraries should update their doctrine/dbal dependency to ^2.13. This PR bumps the doctrine/dbal dependency version and fixes the incompatibility

@acrobat acrobat force-pushed the fix-doctrine-dbal-2.13-compatibility branch from 0e02e58 to 5b219f8 Compare April 5, 2021 17:36
acrobat
acrobat commented Apr 5, 2021
View reviewed changes
@acrobat acrobat force-pushed the fix-doctrine-dbal-2.13-compatibility branch from 5b219f8 to ac5ea0a Compare April 5, 2021 20:00
@bobvandevijver bobvandevijver mentioned this pull request Apr 7, 2021
Closed
@bobvandevijver
Copy link

Maybe it should be ResultStatement instead of Result? See doctrine/dbal#4588

@acrobat
Copy link
Contributor Author

acrobat commented Apr 7, 2021

I think the used interface is correct as the forward compatibility layer also implements Doctrine\DBAL\Result

https://github.com/doctrine/dbal/blob/79a6a843d49de8676efd38f30d29023c69ebff02/lib/Doctrine/DBAL/ForwardCompatibility/Result.php#L21

And in dbal v3 that interface is replaced by the Result class, so this typehint will still work correctly I think.

@OskarStark OskarStark requested a review from wouterj April 9, 2021 14:25
@tarjei tarjei mentioned this pull request Apr 15, 2021
Closed
@acrobat acrobat mentioned this pull request Apr 19, 2021
Closed
@derrabus
Copy link
Member

Thank you @acrobat.

@derrabus derrabus merged commit b4a2199 into symfony:main Apr 20, 2021
@acrobat acrobat deleted the fix-doctrine-dbal-2.13-compatibility branch April 20, 2021 15:03
@acrobat
Copy link
Contributor Author

acrobat commented Apr 22, 2021

Thanks for the merge @derrabus, is it also possible to have this available in a release? Thanks!

@Webonaute
Copy link

I have the same issue? is this has been released?

@acrobat
Copy link
Contributor Author

acrobat commented May 17, 2021

@Webonaute yes, it's included in v3.1.2

@Webonaute
Copy link

I have symfony/security-acl v3.1.2 installed.
I still have the issue. I also have doctrine/dbal 2.13.1 installed.

what else am missing?

@acrobat
Copy link
Contributor Author

acrobat commented May 17, 2021

What exact error do you get?

@Webonaute
Copy link 

 [TypeError]                                                                                                                                                                                                                                                                                    
  Argument 1 passed to Symfony\Component\Security\Acl\Dbal\AclProvider::hydrateObjectIdentities() must be an instance of Doctrine\DBAL\Result, instance of Doctrine\DBAL\Driver\Mysqli\Statement given, called in /var/www/pnp-api/vendor/symfony/security-acl/Dbal/AclProvider.php on line 468  
                                                                                                                                                                                                                                                                                                 

Exception trace:
  at /var/www/pnp-api/vendor/symfony/security-acl/Dbal/AclProvider.php:484
 Symfony\Component\Security\Acl\Dbal\AclProvider->hydrateObjectIdentities() at /var/www/pnp-api/vendor/symfony/security-acl/Dbal/AclProvider.php:468
 Symfony\Component\Security\Acl\Dbal\AclProvider->lookupObjectIdentities() at /var/www/pnp-api/vendor/symfony/security-acl/Dbal/AclProvider.php:176
 Symfony\Component\Security\Acl\Dbal\AclProvider->findAcls() at /var/www/pnp-api/vendor/symfony/security-acl/Dbal/MutableAclProvider.php:128
 Symfony\Component\Security\Acl\Dbal\MutableAclProvider->findAcls() at /var/www/pnp-api/vendor/symfony/security-acl/Dbal/AclProvider.php:92
 Symfony\Component\Security\Acl\Dbal\AclProvider->findAcl() at /var/www/pnp-api/vendor/sonata-project/admin-bundle/src/Security/Handler/AclSecurityHandler.php:167
 Sonata\AdminBundle\Security\Handler\AclSecurityHandler->getObjectAcl() at /var/www/pnp-api/vendor/sonata-project/admin-bundle/src/Util/AdminAclManipulator.php:59
 Sonata\AdminBundle\Util\AdminAclManipulator->configureAcls() at /var/www/pnp-api/vendor/sonata-project/admin-bundle/src/Command/SetupAclCommand.php:70
 Sonata\AdminBundle\Command\SetupAclCommand->execute() at /var/www/pnp-api/vendor/symfony/console/Command/Command.php:255
 Symfony\Component\Console\Command\Command->run() at /var/www/pnp-api/vendor/symfony/console/Application.php:1018
 Symfony\Component\Console\Application->doRunCommand() at /var/www/pnp-api/vendor/symfony/framework-bundle/Console/Application.php:97
 Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() at /var/www/pnp-api/vendor/symfony/console/Application.php:271
 Symfony\Component\Console\Application->doRun() at /var/www/pnp-api/vendor/symfony/framework-bundle/Console/Application.php:83
 Symfony\Bundle\FrameworkBundle\Console\Application->doRun() at /var/www/pnp-api/vendor/symfony/console/Application.php:147
 Symfony\Component\Console\Application->run() at /var/www/pnp-api/bin/console:43

sonata:admin:setup-acl [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-e|--env ENV] [--no-debug] [--trackingid [TRACKINGID]] [--] <command>

@derrabus
Copy link
Member

@Webonaute Can you please produce a reproducer application and open a new issue? That would help us working on a fix. If you can provide a PR with a fix, that would also be more than welcome.

@Webonaute
Copy link

I found out that our internal MasterSlave wrapper was not updated with new PrimaryReplica class and returning wrong object.

@derrabus
Copy link
Member

Okay, so there's nothing to do on our side? Thanks for checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

@jderusse jderusse jderusse approved these changes

@wouterj wouterj Awaiting requested review from wouterj

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Not works with doctrine/dbal v2.13.0
5 participants
@acrobat @bobvandevijver @derrabus @Webonaute @jderusse